Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tgross35 (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Thanks for the feedback, I implemented the suggested changes and will proceed to have the std updated.

@rustbot review

There are five fields in struct flock: l_type(short), l_whence(short), l_start(long long), l_len(long long) and l_pid(long).

Regards,

Hey @biabbas! We must both be looking at a different revision of the same file. See the snippet below of what the fcntlcom.h header contains in the distribution I have available.
I would propose to implement your suggestions regardless, since on versions where only the lower 16-bits are used this should still result in the same behaviour.

/* file segment locking set data type - information passed to system by user */
struct flock {
	short	l_type;		/* F_RDLCK, F_WRLCK, or F_UNLCK */
	short	l_whence;	/* flag to choose starting offset */
	long	l_start;	/* relative offset, in bytes */
	long	l_len;		/* length, in bytes; 0 means lock to EOF */
	short	l_pid;		/* returned with F_GETLK */
	short	l_xxx;		/* reserved for future use */
};

I would propose to implement your suggestions regardless, since on versions where only the lower 16-bits are used this should still result in the same behaviour.

Is this saying there is a change to make here? Just @rustbot review when it's ready for a review again, either with the change or if I misunderstood the comment.

@rustbot author

You are right @tgross35! I committed the final changes for the commenys of biabbas.

@rustbot review

Thanks @jackorobot for authoring and @biabbas for taking a look.

I don't know much about vxworks but is it possible you are looking at headers for 32-bit vs 64-bit? The two mentioned definitions are compatible for the lower four fields on LP64, but not on ILP32.

I'm going to hold off on backporting actually until confirming the above. Would you both mind mentioning what target and vxworks versions the headers you're referencing are from?

If it's a difference in version then we should just add the latest, but I don't want to give 32-bit users unsound API if that's what the difference is.

@tgross35 The snippet I sent earlier is from an VxWorks 7 distribution, which I use on a i686-wrs-vxworks target

I referred the latest vxworks sources, I found only one header file with this definition for for all targets. I also checked the previous vxworks 7 versions but the definitions remained same. I'll check again tomorrow and confirm.

There are five fields in struct flock:
l_type(short), l_whence(short), l_start(long long), l_len(long long) and l_pid(long).

I checked on different targets with the latest source, the struct flock declaration in fcntlcom.h remained the same.

Thanks for checking, this makes no sense. As a last effort to explain things, is it possible to build a program both for i686 and x86_64 and print that? Something like

int main() {
    struct flock f;
    printf(
        "flock %zu, l_type %zu, l_whence %zu, l_start %zu, "
        "l_len %zu, l_pid %zu, long %zu, long long %zu\n",
        sizeof(struct flock),
        sizeof(f.l_type),
        sizeof(f.l_whence),
        sizeof(f.l_start),
        sizeof(f.l_len),
        sizeof(f.l_pid),
        sizeof(long),
        sizeof(long long)
    );
}

VxWorks doesn't do API versioning separate from OS version, do they?

In any case I'll backport this as long as neither of you have objections, but will add a // FIXME(vxworks): ... since we definitely need to get a better answer here at some point.

@tgross35,
On 64 bit targets long long and long are both 8 bytes. So our changes here won't cause any problems as c_long is also 8 bytes.

[vxWorks *]# ./flock.vxe
flock 32
, l_type 2
, l_whence 2
, l_start 8
, l_len 8
, l_pid 8
, long 8
, long long 8

But on 32 bits long long is 8 bytes and long is 4 bytes. c_long is 4 bytes for 32 bit targets. This would indeed cause problems for 32 bit targets.

[vxWorks *]# ./flock.vxe
flock 24
, l_type 2
, l_whence 2
, l_start 8
, l_len 8
, l_pid 4
, long 4
, long long 8

I don't think we should be releasing these changes.

@jackorobot Could you use c_longlong for l_start and l_len.

I am not sure what to tell you other than that the struct I based this on the version of fcntlcom.h I have available, and (as shown in the snippet I sent) the types defined for l_start and l_len in that version is long, and not long long.

Maybe it is because the niche platform I am looking at, and if @biabbas has more versions that confirm the long long datatypes, then I agree to use his definition and switch to long long.

Are there more specific version numbers? Aiui vxworks 7 has been around since 2010

I have access to almost all VxWorks versions. I checked the early VxWorks 7versions and VxWorks 6 versions too, but the definition was the same as in the latest 24.03 version.
@jackorobot Do you know the version of the VxWorks workbench you are using?

@biabbas It is 23.09. But it is supplied to us by some equipment manufacturer, and I would not be surprised if they (for backwards compatibility) supply very old header file versions (with dates from 1999 etc...)

Not exactly. It is still in the sys/fcntlcom.h folder, relative to the root system include folder. Again, I think I am looking at a weird platform (even for VxWorks standards), and lets agree to have the version that is provided by windriver directly.